image: constrain axes to domain, apply defaults whenever an image is present #4313
Conversation
antoinerg
changed the title
antoinerg
changed the title
| if(axLetter === 'y' && !axLayoutIn.hasOwnProperty('scaleanchor') && axHasImage[axName]) { | ||
| scaleanchorDflt = axLayoutOut.anchor; | ||
| } | ||
|
|
||
| var constrainDflt = null; | ||
| var constrainDflt; |
There was a problem hiding this comment.
I think we need to reset to null as we're in a loop:
var axNames = ['xaxis', 'yaxis']
for(var i = 0; i < axNames.length; i++) {
var dflt;
if(axNames[i] === 'xaxis') {
dflt = 'NEW DEFAULT';
}
console.log(dflt)
}prints NEW DEFAULT twice, whereas
var axNames = ['xaxis', 'yaxis']
for(var i = 0; i < axNames.length; i++) {
var dflt = null;
if(axNames[i] === 'xaxis') {
dflt = 'NEW DEFAULT';
}
console.log(dflt)
}print NEW DEFAULT and then null.
There was a problem hiding this comment.
You're right but setting it to null will prevent var constrain = coerce('constrain', constrainDflt); from getting the default value. This can be seen in the previous commit d1eca2c for which test-image fails. Any suggestion/simple fix for this?
There was a problem hiding this comment.
Setting it to undefined make the tests pass but the linter complains :\
There was a problem hiding this comment.
The following pass all the tests (including linter). Would that be 👌 ?
var scaleanchorDflt;
if(axLetter === 'y' && !axLayoutIn.hasOwnProperty('scaleanchor') && axHasImage[axName]) {
scaleanchorDflt = axLayoutOut.anchor;
} else {scaleanchorDflt = undefined;}
var constrainDflt;
if(!axLayoutIn.hasOwnProperty('constrain') && axHasImage[axName]) {
constrainDflt = 'domain';
} else {constrainDflt = undefined;}There was a problem hiding this comment.
What about:
var scaleanchorDflt = undefined;
if(axLetter === 'y' && !axLayoutIn.hasOwnProperty('scaleanchor') && axHasImage[axName]) {
scaleanchorDflt = axLayoutOut.anchor;
}
var constrainDflt = undefined;
if(!axLayoutIn.hasOwnProperty('constrain') && axHasImage[axName]) {
constrainDflt = 'domain';
}??
There was a problem hiding this comment.
Ha that's annoying. Your latest is fine 👍
There was a problem hiding this comment.
by latest I mean:
var scaleanchorDflt;
if(axLetter === 'y' && !axLayoutIn.hasOwnProperty('scaleanchor') && axHasImage[axName]) {
scaleanchorDflt = axLayoutOut.anchor;
} else {
scaleanchorDflt = undefined;
}
var constrainDflt;
if(!axLayoutIn.hasOwnProperty('constrain') && axHasImage[axName]) {
constrainDflt = 'domain';
} else {
constrainDflt = undefined;
}
|
Awesome - 💃 let's get this in #4307 |
Following #4307 (comment), this PR adds a smart default layout to constrain x and y axes to the domain if there is an image present.
The notion of a "smart default layout" is changed slightly in this PR for
image. Now, the layout defaults will be applied to a subplot's axes whenever animageis present in the subplot. Adding a scatter on top will not affect the defaults. This is clearly tested in a very explicit (and probably redundant) series of jasmine tests.